Refactor: Convert chunks_exact[_mut] iterators to as_chunks[_mut] iterators#2765
Refactor: Convert chunks_exact[_mut] iterators to as_chunks[_mut] iterators#2765197g merged 15 commits intoimage-rs:mainfrom
chunks_exact[_mut] iterators to as_chunks[_mut] iterators#2765Conversation
Use the `as_chunks[_mut]` iterators when the chunk size is constant Replace iterators in `codecs/farbfeld.rs` Change `consume_channel`'s signature to use an array
okaneco
force-pushed
the
convert_to_as_chunks
branch
2 times, most recently
from
d9435e4 to
031cb3b
Compare
okaneco
force-pushed
the
convert_to_as_chunks
branch
from
031cb3b to
1d1bb95
Compare
197g
left a comment
197g
left a comment
There was a problem hiding this comment.
Other than a few style nits, this looks good. Thank you. Could you have a cursory look if you see the redundant &mut chunk[..CN] pattern with other constants elsewhere? Nothing crucial but if you find them then the style could be cleared up.
src/imageops/filter_1d.rs
Outdated
| for (x, dst) in (image_size.width..(image_size.width + pad_w)) | ||
| .zip(row_buffer.as_chunks_mut::<N>().0.iter_mut().rev()) | ||
| { | ||
| let old_x = x.max(0).min(image_size.width - 1); |
There was a problem hiding this comment.
Wondering why max(0) on usize isn't triggering clippy::unnecessary_min_or_max.
There was a problem hiding this comment.
I believe .max(0) is a clippy bug and can be safely removed. Interestingly, this presents in my original code for years.
There was a problem hiding this comment.
Just pushed a commit to remove it.
| let bias_y = range.bias_y as i32; | ||
| let bias_uv = range.bias_uv as i32; | ||
| let y_iter = y_plane.chunks_exact(2); | ||
| let rgb_chunks = rgba.chunks_exact_mut(CHANNELS * 2); |
There was a problem hiding this comment.
Interesting fact: Rust with LLVM was always bad in autovectorizing YUV chroma subsampling.
Now, with as_chunks[_mut] it can at least autovectorize YUV encoding with chroma subsampling but not decoding.
Remove pattern of using slicing to do one bounds check now that arrays are being used. Break array::map function into its own line Write directly into array instead of indexing since the types match Remove no-op `max(0)` call on a usize
okaneco
force-pushed
the
convert_to_as_chunks
branch
from
ca89210 to
e6dad12
Compare
|
Thank you! |
3 participants
as_chunks[_mut]iterators when the chunk size is constant.There's a clippy lint for this in the works but it's not merged yet.
This is a refactoring PR with no functional changes intended.
The newer chunk iterators give the compiler more information to potentially eliminate bounds checks or autovectorize/unroll beyond what the old style iterators could do. There are still cases today, with the latest LLVM upgrade, where simple-looking
chunks_exactcode doesn't autovectorize until upgraded to useas_chunksiterators. The new iterators are also more resilient to autovectorizer regressions such as the ones we saw starting in 1.87 (LLVM 20) which were fixed recently with the bump to LLVM 22.This allowed for some other local cleanups, such as:
try_into().unwrap()array casts